-
Notifications
You must be signed in to change notification settings - Fork 709
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add write to disk options for build command #12006
Conversation
Build Artifacts
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the diff can be a bit smaller, and no need to disable the cache in this case that I can see.
packages/kolibri-tools/lib/cli.js
Outdated
); | ||
runWebpackBuild(mode, bundleData, false, { | ||
...options, | ||
cache: false, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason not to utilize the webpack cache here? It should help with build performance either way.
Also, the use of hot is limited to the devserver being active, which we are passing false for, so I think this if
could do the warn, then we could update the runWebpackBuild to simply be:
runWebpackBuild(mode, bundleData, !options.writeToDisk && mode === modes.DEV, options);
and not have to invoke it separately in the if statement.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, I thought cache will fill the dev's system
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So yes, if hot and cache are not being modified then i will run the webpack build with your suggestions. Thank You
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The cache is better managed than the writing built files to disk, so I wouldn't worry too much!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done!! Thank You
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent, thank you!
Summary
This exposes
--write-to-disk
flag in build that writes file to disk instead of webpackReferences
Fixes #4826
Reviewer guidance
In https://github.com/learningequality/kolibri/blob/develop/package.json#L15 Should there be a
prod
argument?Testing checklist
PR process
Reviewer checklist
yarn
andpip
)